Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Use nested checkboxes and radio buttons all over admin #2429

Merged
merged 7 commits into from
Dec 20, 2017

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Dec 4, 2017

Following our own styleguide and due to better styleability we
should nest checkboxes and radio buttons into their labels.

This also removes some redundant and unused style definitions
regarding labels, checkboxes and radio buttons.

The most debatable change might be the label font-size change (from 90% to 100%)

Before

orders labels before

After

orders labels after

These are not used anywhere
The bootstrap label style has better readability
For radios and checkboxes nested in label tags we want to add some small right margin to enhance readability
We want the checkboxes to be vertically aligned to the text fields.
Checkboxes should be nested in label tags, so we can style them better
Most of our radio buttons are already nested in label tags. This converts the last two left overs.
@tvdeyen tvdeyen added changelog:solidus_backend Changes to the solidus_backend gem Code review needed labels Dec 4, 2017
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion on font size increase, but I'm 👍 if you think it's ok.

If someone else has my same doubts about removing for attribute from labels, it's perfectly valid and right. Label without for will be associated to all its contents. 👍

Thanks!

@jhawthorn jhawthorn merged commit b9e6057 into solidusio:master Dec 20, 2017
@tvdeyen tvdeyen deleted the label-styles branch February 8, 2019 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants